Conversation
WalkthroughThe PR introduces Evolve configuration support to the devnet genesis system by adding a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes introduce new wrapper types and refactor existing APIs across multiple files with custom JSON marshaling logic. While the structural modifications are straightforward, understanding the type hierarchy, verifying the JSON serialization correctness, and confirming API compatibility requires moderate analytical effort. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
genutil/types.go (1)
32-74: Harden MarshalJSON: require non-nil Config and default empty alloc map.Avoid emitting "config": null or "alloc": null in edge paths. Fail fast if Config is missing and serialize empty alloc as {}.
Apply this diff:
--- a/genutil/types.go +++ b/genutil/types.go @@ import ( "encoding/json" + "fmt" @@ func (g GenesisWithEvolve) MarshalJSON() ([]byte, error) { type Genesis struct { @@ } var enc Genesis - enc.Config = g.Config + enc.Config = g.Config + if enc.Config == nil { + return nil, fmt.Errorf("genesis config is required") + } @@ - if g.Alloc != nil { - enc.Alloc = make(map[common.UnprefixedAddress]coretypes.Account, len(g.Alloc)) - for k, v := range g.Alloc { - enc.Alloc[common.UnprefixedAddress(k)] = v - } - } + if g.Alloc == nil { + enc.Alloc = map[common.UnprefixedAddress]coretypes.Account{} + } else { + enc.Alloc = make(map[common.UnprefixedAddress]coretypes.Account, len(g.Alloc)) + for k, v := range g.Alloc { + enc.Alloc[common.UnprefixedAddress(k)] = v + } + }Optionally, add ,omitempty to BaseFee/ExcessBlobGas/BlobGasUsed if you prefer to omit nulls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
devnet/applayer/genesis.json(1 hunks)genutil/main.go(1 hunks)genutil/types.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
genutil/types.go (1)
genutil/main.go (1)
ChainConfig(15-48)
genutil/main.go (1)
genutil/types.go (4)
ChainConfigWithEvolve(15-18)EvolveChainConfig(27-30)GenesisWithEvolve(21-24)Genesis(35-51)
🔇 Additional comments (6)
genutil/types.go (1)
14-30: Wrapper types for Evolve support are clear and minimal.Struct embedding and JSON tags look good; the optional Evolve block matches the intended schema.
genutil/main.go (4)
15-47: ChainConfig wiring LGTM.Defaults match the committed JSON (IDs, hardfork heights/times, blob schedule). Evolve.MintAdmin plumbs through cleanly.
52-59: Genesis header fields align with genesis.json.GasLimit 60,000,000 and Timestamp 1,758,708,000 (0x68d3c120) match the JSON. No issues.
81-82: Config is sourced from ChainConfig(Devnet, &authority).Good — this prevents drift between alloc’s authority and evolve.mintAdmin.
66-76: This review comment is factually incorrect about the Solidity short-string encoding specification.For Solidity short strings (≤31 bytes), the least significant byte should be
len*2with the low bit = 0, notlen*2 + 1. Thelen*2 + 1encoding (with low bit = 1) is reserved for long strings (>31 bytes).The current implementation in
genutil/main.go:147is correct:value[31] = byte(len(bz) * 2)For your example: "Wrapped Noble" (13 bytes) →
13*2 = 0x1a(binary:0001_1010, low bit = 0) is the correct encoding. Setting it to0x1b(low bit = 1) would incorrectly signal a long string layout.The codebase requires no changes. Disregard the suggested refactoring.
Likely an incorrect or invalid review comment.
devnet/applayer/genesis.json (1)
33-36: All verifications passed — addresses are consistent across config, code, and alloc.The checks confirm:
config.evolve.mintAdminmatches theauthorityaddress in codeallocincludes the authority address (unprefixed) as a keyNo issues found.
1 participant
Summary by CodeRabbit